Improve diagnostics for StructExpression
#4357
Conversation
kfcripps
force-pushed
the
improve-diagnostics-structexpr
branch
from
a67d4a8
to
bcde4cd
Compare
|
I am not sure why a cstring could be null here. Let me debug this. |
Thanks! |
There was a problem hiding this comment.
I found the reason for the crash. No problem with your code but there is a problem with the way cstrings are initialized.
If you just declare them cstring var; the internal character will be a nullptr and the cstring will be invalid. I think this is something that should be fix, not sure why the class is written this way.
Either way, applying this patch should fix the problem:
diff --git a/backends/p4tools/p4tools.def b/backends/p4tools/p4tools.def
index ee2066443..aec5bb13d 100644
--- a/backends/p4tools/p4tools.def
+++ b/backends/p4tools/p4tools.def
@@ -211,7 +211,7 @@ public:
inline cstring concolicMethodName = "";
toString {
- cstring argumentStr;
+ cstring argumentStr = "";
cstring sep = "";
for (const auto *arg : *arguments) {
argumentStr += sep + arg->toString();
ir/expression.def
Outdated
| @@ -482,6 +482,7 @@ class StructExpression : Expression { | |||
| size_t size = components.size(); | |||
| return components.at(size - 1)->is<IR::NamedDots>(); | |||
| } | |||
| cstring toString() const; | |||
There was a problem hiding this comment.
This should either be marked override or implemented in the form of
toString {
cstring str = "{";
if (!components.empty()) {
cstring exprStr = components.at(0)->expression->toString();
str += " " + components.at(0)->toString() + " = " + exprStr;
}
for (unsigned i = 1; i < size(); i++) {
cstring exprStr = components.at(i)->expression->toString();
str += ", " + components.at(i)->toString() + " = " + exprStr;
}
return str + " }";
}like with other nodes.
kfcripps
force-pushed
the
improve-diagnostics-structexpr
branch
from
bcde4cd
to
a97cbd3
Compare
kfcripps
force-pushed
the
improve-diagnostics-structexpr
branch
2 times, most recently
from
05343d1
to
a97cbd3
Compare
|
@kfcripps Are you able to enable automerge for these PRs? If not, I can toggle it. |
|
@fruffy How do I do that? |
|
The CI has this option, but I believe you need merge rights for this. |
|
Yeah, I've never contributed here before, so I don't have any rights in this repo. |
|
I can enable it for the PRs, if you want me to. |
|
Would that enable auto-merging for all future PRs, or do you have to enable it individually for each? |
|
It is for each PR separately. In general when you have no write access, you can ask someone to merge the PR for you once you are happy with it and it is approved. In its current state it will probably have to be updated/rebased before that can be done (you will probably see a button for that). |
|
Ok, I don't see why not - please enable it for my PRs. Thanks |
kfcripps
force-pushed
the
improve-diagnostics-structexpr
branch
from
a97cbd3
to
145229d
Compare
|
Rebased |
|
Looks like the build_indirect job is stuck? |
No description provided.